trait_selection: fix type-const eval ICE#152040
trait_selection: fix type-const eval ICE#152040JohnTitor wants to merge 2 commits intorust-lang:mainfrom
Conversation
rustbot
added
S-waiting-on-review
|
|
| if !type_const_stack.insert(uv.def) { | ||
| let guar = tcx.dcx().span_err( | ||
| tcx.def_span(uv.def), | ||
| "cycle detected when evaluating type-level constant", |
There was a problem hiding this comment.
I'm not sure if we should detect a cyclic error in this place though, maybe we can handle it better?
There was a problem hiding this comment.
I'm don't know how/where/when we invoke try_evaluate_const (during normalization of course, but …) but ideally (?) either the query system would detect cycles / overflows for us (but I guess no queries are involved here) or we have a simple depth param similar to what type normalization does instead of storing visited definitions. idk, imma wait for Boxy
|
Does this code compiles under your impl? |
JohnTitor
force-pushed
the
issue-151631
branch
from
3fc70e5 to
e853189
Compare
This comment has been minimized.
This comment has been minimized.
|
@Human9000-bit Yeah, it should. Added a rev so that we can make sure it. |
This comment has been minimized.
This comment has been minimized.
|
|
||
| trait NewTrait {} | ||
|
|
||
| impl NewTrait for () {} |
There was a problem hiding this comment.
By adding this impl, this test doesn't reproduce the ICE on nightly/main.
| impl NewTrait for () {} |
| #![feature(generic_const_items, min_generic_const_args)] | ||
| #![feature(adt_const_params)] | ||
| #![allow(incomplete_features)] | ||
| #![allow(dead_code)] |
There was a problem hiding this comment.
This is implied in UI tests.
| #![allow(dead_code)] |
There was a problem hiding this comment.
This test isn't minimized and contains a lot of unrelated elements. Here's a smaller one:
// issue: <https://github.com/rust-lang/rust/issues/151631>
//@ compile-flags: -Znext-solver
#![feature(min_generic_const_args)]
#![expect(incomplete_features)]
trait SuperTrait {}
trait Trait: SuperTrait {
type const K: u32;
}
impl Trait for () {
type const K: u32 = const { 1 };
}
fn check(_: impl Trait<K = 0>) {}
fn main() {
check(());
}There was a problem hiding this comment.
Ah, I tweaked during developing then forgot to revert. Thank you! Fixed in 1641602.
| } | ||
| }; | ||
|
|
||
| if tcx.is_type_const(uv.def) { |
There was a problem hiding this comment.
I don't know if this change makes sense, I'm not an expert in mGCA. Therefore I'll assign @BoxyUwU instead.
| trait Owner { | ||
| #[type_const] | ||
| const C<const N: u32>: u32; | ||
| //~^ ERROR cycle detected when evaluating type-level constant |
There was a problem hiding this comment.
Ideally this error would point to the C in the trait impl.
| if !type_const_stack.insert(uv.def) { | ||
| let guar = tcx.dcx().span_err( | ||
| tcx.def_span(uv.def), | ||
| "cycle detected when evaluating type-level constant", |
There was a problem hiding this comment.
I'm don't know how/where/when we invoke try_evaluate_const (during normalization of course, but …) but ideally (?) either the query system would detect cycles / overflows for us (but I guess no queries are involved here) or we have a simple depth param similar to what type normalization does instead of storing visited definitions. idk, imma wait for Boxy
JohnTitor
force-pushed
the
issue-151631
branch
from
e853189 to
1641602
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
5 participants
Fixes #151631, fixes #151477
r? @fmease
I'd recommend reviewing commit-by-commit, the diff is less-readable to address a cyclic issue.